Conversation
horsley
left a comment
There was a problem hiding this comment.
TLS Configuration (Lines 106-118): The variables TLSCA, TLSCert, and TLSKey are checked but never actually used. The code has empty comment blocks where the certificate loading logic should be implemented. You need to implement actual certificate loading using tls.LoadX509KeyPair or similar.
horsley
left a comment
There was a problem hiding this comment.
Redundant Health Check (Lines 206-232): The paho.mqtt.golang library already has built-in automatic reconnection. Looking at lines 79-84, you set SetAutoReconnect(true), SetConnectRetry(true), SetConnectRetryInterval(5s), and SetMaxReconnectInterval(5m). The manual reconnection logic in startHealthCheck() is redundant and may cause race conditions with the library's internal reconnection mechanism. Consider removing the startHealthCheck function or at least the manual reconnection code.
| // TLS configuration | ||
| if c.config.TLS { | ||
| tlsConfig := &tls.Config{ | ||
| InsecureSkipVerify: false, |
There was a problem hiding this comment.
Issue 1: Incomplete TLS Implementation
The TLS certificate loading code (around line 86) has comments but no actual implementation. The code creates a tls.Config but never loads the certificates from the configured paths (TLSCA, TLSCert, TLSKey). Consider implementing the certificate loading logic.
pkg/channels/mqtt/mqtt.go
Outdated
| } | ||
|
|
||
| // startHealthCheck starts a periodic health check for the MQTT connection. | ||
| func (c *MQTTChannel) startHealthCheck() { |
There was a problem hiding this comment.
Issue 2: Redundant Health Check
The paho.mqtt library already has built-in automatic reconnection with the options SetAutoReconnect(true), SetConnectRetry(true), SetMaxReconnectInterval(5 * time.Minute) configured. The manual health check (startHealthCheck) and reconnection logic may cause race conditions with the library's internal reconnection mechanism. Consider removing the manual health check and relying solely on the library's built-in reconnection.
|
@avaksru LGTM, try rebase main branch to resolve conflicts :) |
|
@avaksru plz resolve conflicts |
|
@avaksru there's still has conflicts, plz resolve conflicts |
9aa2361 to
610e8ad
Compare
- Fix pkg/gateway/gateway.go: remove unnecessary leading/trailing newlines and fix gofumpt formatting - Fix pkg/config/config.go: reformat long struct tags to meet golines 120-char limit - Fix pkg/channels/mqtt/mqtt.go: fix gofmt formatting - Fix gci import ordering across pkg/config/ and pkg/channels/mqtt/ packages All linter errors resolved: 0 issues remaining
📝 Description
Implemented periodic MQTT connection health check with automatic reconnection. Added
startHealthCheck()method that monitors connection status every 30 seconds and automatically reconnects when connection is lost. The system continues to operate normally even when the MQTT server is unavailable, without interfering with other application functionality.🗣️ Type of Change
🤖 AI Code Generation
🔗 Related Issue
No related issue
📚 Technical Context (Skip for Docs)
🧪 Test Environment
📸 Evidence (Optional)
Click to view Logs/Screenshots
Key code changes:
Implementation features:
☑️ Checklist
Ready to switch to Act mode to apply these changes to the documentation if needed.